From c1c8e361ce23ef323921894b2c2cac03aec89e9c Mon Sep 17 00:00:00 2001 From: tee-too Date: Mon, 20 Mar 2017 11:39:22 +0100 Subject: [PATCH] Report the name of the test that failed (fix #2529) --- src/bin/test.rs | 2 +- src/cargo/ops/cargo_rustc/compilation.rs | 4 +- src/cargo/ops/cargo_rustc/mod.rs | 1 + src/cargo/ops/cargo_test.rs | 35 ++++----- src/cargo/util/errors.rs | 30 +++++++- src/cargo/util/mod.rs | 2 +- tests/test.rs | 91 +++++++++++++++++++++++- 7 files changed, 141 insertions(+), 24 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index afd482cd6..65f9b4482 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -149,7 +149,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult { None => Ok(()), Some(err) => { Err(match err.exit.as_ref().and_then(|e| e.code()) { - Some(i) => CliError::new(human("test failed"), i), + Some(i) => CliError::new(human(err.hint()), i), None => CliError::new(Box::new(Human(err)), 101), }) } diff --git a/src/cargo/ops/cargo_rustc/compilation.rs b/src/cargo/ops/cargo_rustc/compilation.rs index 9f9b1c52e..37bf4008f 100644 --- a/src/cargo/ops/cargo_rustc/compilation.rs +++ b/src/cargo/ops/cargo_rustc/compilation.rs @@ -3,7 +3,7 @@ use std::ffi::OsStr; use std::path::PathBuf; use semver::Version; -use core::{PackageId, Package, Target}; +use core::{PackageId, Package, Target, TargetKind}; use util::{self, CargoResult, Config, ProcessBuilder, process, join_paths}; /// A structure returning the result of a compilation. @@ -13,7 +13,7 @@ pub struct Compilation<'cfg> { pub libraries: HashMap>, /// An array of all tests created during this compilation. - pub tests: Vec<(Package, String, PathBuf)>, + pub tests: Vec<(Package, TargetKind, String, PathBuf)>, /// An array of all binaries created. pub binaries: Vec, diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 0cac00a95..0675f095c 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -147,6 +147,7 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>, if unit.profile.test { cx.compilation.tests.push((unit.pkg.clone(), + unit.target.kind().clone(), unit.target.name().to_string(), dst)); } else if unit.target.is_bin() || unit.target.is_example() { diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index c8abc0ed9..1de437090 100644 --- a/src/cargo/ops/cargo_test.rs +++ b/src/cargo/ops/cargo_test.rs @@ -1,7 +1,7 @@ use std::ffi::{OsString, OsStr}; use ops::{self, Compilation}; -use util::{self, CargoResult, CargoTestError, ProcessError}; +use util::{self, CargoResult, CargoTestError, Test, ProcessError}; use core::Workspace; pub struct TestOptions<'a> { @@ -19,7 +19,7 @@ pub fn run_tests(ws: &Workspace, if options.no_run { return Ok(None) } - let mut errors = if options.only_doc { + let (test, mut errors) = if options.only_doc { run_doc_tests(options, test_args, &compilation)? } else { run_unit_tests(options, test_args, &compilation)? @@ -27,7 +27,7 @@ pub fn run_tests(ws: &Workspace, // If we have an error and want to fail fast, return if !errors.is_empty() && !options.no_fail_fast { - return Ok(Some(CargoTestError::new(errors))) + return Ok(Some(CargoTestError::new(test, errors))) } // If a specific test was requested or we're not running any tests at all, @@ -35,15 +35,16 @@ pub fn run_tests(ws: &Workspace, if let ops::CompileFilter::Only { .. } = options.compile_opts.filter { match errors.len() { 0 => return Ok(None), - _ => return Ok(Some(CargoTestError::new(errors))) + _ => return Ok(Some(CargoTestError::new(test, errors))) } } - errors.extend(run_doc_tests(options, test_args, &compilation)?); + let (doctest, docerrors) = run_doc_tests(options, test_args, &compilation)?; + errors.extend(docerrors); if errors.is_empty() { Ok(None) } else { - Ok(Some(CargoTestError::new(errors))) + Ok(Some(CargoTestError::new(doctest, errors))) } } @@ -57,10 +58,10 @@ pub fn run_benches(ws: &Workspace, if options.no_run { return Ok(None) } - let errors = run_unit_tests(options, &args, &compilation)?; + let (test, errors) = run_unit_tests(options, &args, &compilation)?; match errors.len() { 0 => Ok(None), - _ => Ok(Some(CargoTestError::new(errors))), + _ => Ok(Some(CargoTestError::new(test, errors))), } } @@ -69,7 +70,7 @@ fn compile_tests<'a>(ws: &Workspace<'a>, -> CargoResult> { let mut compilation = ops::compile(ws, &options.compile_opts)?; compilation.tests.sort_by(|a, b| { - (a.0.package_id(), &a.1).cmp(&(b.0.package_id(), &b.1)) + (a.0.package_id(), &a.2).cmp(&(b.0.package_id(), &b.2)) }); Ok(compilation) } @@ -78,13 +79,13 @@ fn compile_tests<'a>(ws: &Workspace<'a>, fn run_unit_tests(options: &TestOptions, test_args: &[String], compilation: &Compilation) - -> CargoResult> { + -> CargoResult<(Test, Vec)> { let config = options.compile_opts.config; let cwd = options.compile_opts.config.cwd(); let mut errors = Vec::new(); - for &(ref pkg, _, ref exe) in &compilation.tests { + for &(ref pkg, ref kind, ref test, ref exe) in &compilation.tests { let to_display = match util::without_prefix(exe, cwd) { Some(path) => path, None => &**exe, @@ -101,23 +102,23 @@ fn run_unit_tests(options: &TestOptions, if let Err(e) = cmd.exec() { errors.push(e); if !options.no_fail_fast { - break + return Ok((Test::UnitTest(kind.clone(), test.clone()), errors)) } } } - Ok(errors) + Ok((Test::Multiple, errors)) } fn run_doc_tests(options: &TestOptions, test_args: &[String], compilation: &Compilation) - -> CargoResult> { + -> CargoResult<(Test, Vec)> { let mut errors = Vec::new(); let config = options.compile_opts.config; // We don't build/rust doctests if target != host if config.rustc()?.host != compilation.target { - return Ok(errors); + return Ok((Test::Doc, errors)); } let libs = compilation.to_doc_test.iter().map(|package| { @@ -179,10 +180,10 @@ fn run_doc_tests(options: &TestOptions, if let Err(e) = p.exec() { errors.push(e); if !options.no_fail_fast { - return Ok(errors); + return Ok((Test::Doc, errors)); } } } } - Ok(errors) + Ok((Test::Doc, errors)) } diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs index 14697ad8e..a7b7d7558 100644 --- a/src/cargo/util/errors.rs +++ b/src/cargo/util/errors.rs @@ -8,6 +8,8 @@ use std::process::{Output, ExitStatus}; use std::str; use std::string; +use core::TargetKind; + use curl; use git2; use handlebars; @@ -139,13 +141,20 @@ impl fmt::Debug for ProcessError { /// Error when testcases fail pub struct CargoTestError { + pub test: Test, pub desc: String, pub exit: Option, pub causes: Vec, } +pub enum Test { + Multiple, + Doc, + UnitTest(TargetKind, String) +} + impl CargoTestError { - pub fn new(errors: Vec) -> Self { + pub fn new(test: Test, errors: Vec) -> Self { if errors.is_empty() { panic!("Cannot create CargoTestError from empty Vec") } @@ -153,11 +162,30 @@ impl CargoTestError { .collect::>() .join("\n"); CargoTestError { + test: test, desc: desc, exit: errors[0].exit, causes: errors, } } + + pub fn hint(&self) -> String { + match &self.test { + &Test::UnitTest(ref kind, ref name) => { + match *kind { + TargetKind::Bench => format!("test failed, to rerun pass '--bench {}'", name), + TargetKind::Bin => format!("test failed, to rerun pass '--bin {}'", name), + TargetKind::Lib(_) => "test failed, to rerun pass '--lib'".into(), + TargetKind::Test => format!("test failed, to rerun pass '--test {}'", name), + TargetKind::ExampleBin | TargetKind::ExampleLib(_) => + format!("test failed, to rerun pass '--example {}", name), + _ => "test failed.".into() + } + }, + &Test::Doc => "test failed, to rerun pass '--doc'".into(), + _ => "test failed.".into() + } + } } impl fmt::Display for CargoTestError { diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index ccf013ec5..554f4f004 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -1,7 +1,7 @@ pub use self::cfg::{Cfg, CfgExpr}; pub use self::config::{Config, homedir}; pub use self::dependency_queue::{DependencyQueue, Fresh, Dirty, Freshness}; -pub use self::errors::{CargoResult, CargoError, ChainError, CliResult}; +pub use self::errors::{CargoResult, CargoError, Test, ChainError, CliResult}; pub use self::errors::{CliError, ProcessError, CargoTestError}; pub use self::errors::{Human, caused_human}; pub use self::errors::{process_error, internal_error, internal, human}; diff --git a/tests/test.rs b/tests/test.rs index 2f0d3e633..f55c0bc98 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -172,7 +172,7 @@ fn many_similar_names() { } #[test] -fn cargo_test_failing_test() { +fn cargo_test_failing_test_in_bin() { let p = project("foo") .file("Cargo.toml", &basic_bin_manifest("foo")) .file("src/foo.rs", r#" @@ -200,7 +200,7 @@ fn cargo_test_failing_test() { [COMPILING] foo v0.5.0 ({url}) [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] [RUNNING] target[/]debug[/]deps[/]foo-[..][EXE] -[ERROR] test failed", url = p.url())) +[ERROR] test failed, to rerun pass '--bin foo'", url = p.url())) .with_stdout_contains(" running 1 test test test_hello ... FAILED @@ -221,6 +221,93 @@ test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured .with_status(101)); } +#[test] +fn cargo_test_failing_test_in_test() { + let p = project("foo") + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("src/foo.rs", r#" + pub fn main() { + println!("hello"); + }"#) + .file("tests/footest.rs", r#" + #[test] + fn test_hello() { + assert!(false) + }"#); + + assert_that(p.cargo_process("build"), execs().with_status(0)); + assert_that(&p.bin("foo"), existing_file()); + + assert_that(process(&p.bin("foo")), + execs().with_status(0).with_stdout("hello\n")); + + assert_that(p.cargo("test"), + execs().with_stderr(format!("\ +[COMPILING] foo v0.5.0 ({url}) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +[RUNNING] target[/]debug[/]deps[/]foo-[..][EXE] +[RUNNING] target[/]debug[/]deps[/]footest-[..][EXE] +[ERROR] test failed, to rerun pass '--test footest'", url = p.url())) + .with_stdout_contains(" +running 0 tests + +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured + + +running 1 test +test test_hello ... FAILED + +failures: + +---- test_hello stdout ---- +thread 'test_hello' panicked at 'assertion failed: false', \ + tests[/]footest.rs:4 +") + .with_stdout_contains("\ +failures: + test_hello + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured +") + .with_status(101)); +} + +#[test] +fn cargo_test_failing_test_in_lib() { + let p = project("foo") + .file("Cargo.toml", &basic_lib_manifest("foo")) + .file("src/lib.rs", r#" + #[test] + fn test_hello() { + assert!(false) + }"#); + + assert_that(p.cargo_process("test"), + execs().with_stderr(format!("\ +[COMPILING] foo v0.5.0 ({url}) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +[RUNNING] target[/]debug[/]deps[/]foo-[..][EXE] +[ERROR] test failed, to rerun pass '--lib'", url = p.url())) + .with_stdout_contains(" +running 1 test +test test_hello ... FAILED + +failures: + +---- test_hello stdout ---- +thread 'test_hello' panicked at 'assertion failed: false', \ + src[/]lib.rs:4 +") + .with_stdout_contains("\ +failures: + test_hello + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured +") + .with_status(101)); +} + + #[test] fn test_with_lib_dep() { let p = project("foo") -- 2.30.2